Skip to content

Conversation

@pkedy
Copy link
Member

@pkedy pkedy commented Oct 18, 2021

Description

This PR adds the initial certification testing framework. This is intended to be a starting point and evolve over time as requirements evolve.

Note to reviewers: Go docs/comments will be added to this in a future PR. The main motivation for this limited PR now is to get this in the hands of developers that are looking to start using it now knowing that were will be questions. I recommend reaching out on Discord for help.

Checklist

@pkedy pkedy requested review from a team as code owners October 18, 2021 22:35
@pkedy pkedy changed the title Certification testing poc Initial certification testing "framework" Oct 18, 2021

daprHTTPPort = runtime.DefaultDaprHTTPPort
daprAPIGRPCPort = runtime.DefaultDaprAPIGRPCPort
daprInternalGRPC = runtime.DefaultDaprAPIGRPCPort + 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we do not have a constant value for internal grpc port?

/cc @artursouza @yaron2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is constant. I don't understand the question.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he is asking why there is not a constant for the internal port in dapr/dapr's runtime package. That struck me as strange also. Is the port dynamically selected (which would since)?

return c.Context.Value(key)
}
func (c Context) MustGet(args ...interface{}) {
if len(args)%2 != 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to comment magic number here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be explained in the forthcoming Go docs.

daixiang0
daixiang0 previously approved these changes Oct 19, 2021
Copy link
Member

@daixiang0 daixiang0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, just some nits.

Copy link
Contributor

@CodeMonkeyLeet CodeMonkeyLeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ... are there issues tracking follow-up tasks to this? (e.g. doc on usage, update to dev instructions, integration into github workflow, update devcontainer with terraform etc.)

@halspang @tanvigour @berndverst @dmitsh @mukundansundar, given that this PR is intended to unblock devs on implementing integration tests while the docs are still incoming, you should take a look.

@@ -0,0 +1,71 @@
package terraform
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we officially introducing terraform as a dependency into our test environments? Would be good to update the developing-component.md doc, and we should probably add it to the devcontainer definition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at this time, but the thought was that it could be used to provision various cloud services in the future.

@codecov
Copy link

codecov bot commented Oct 20, 2021

Codecov Report

Merging #1204 (c07d6b2) into master (029ca1d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1204   +/-   ##
=======================================
  Coverage   35.07%   35.07%           
=======================================
  Files         147      147           
  Lines       12624    12624           
=======================================
  Hits         4428     4428           
  Misses       7727     7727           
  Partials      469      469           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32d6083...c07d6b2. Read the comment docs.

@artursouza artursouza merged commit 698ffb5 into dapr:master Oct 20, 2021
@pkedy pkedy deleted the certification_testing_poc branch October 20, 2021 17:59
amimimor pushed a commit to amimimor/components-contrib that referenced this pull request Dec 9, 2021
* WIP

* tweaks

* Simplifying runnables

* Increase network timeout

* tweaks

* More tweaks

* Docker compose and structural tweaks

Co-authored-by: tanvigour <60332928+tanvigour@users.noreply.github.com>

* Tweak

* Working Kafka test

* Watcher ordered vs unordered

* Error simulation

* Made port explicit, removed Redis

* package.Run (better naming)

* terraform related changes

* Restructuring certification testing

* Remove copied go-sdk client package

* Fix terraform.go build issues

* Using master of go-sdk

* Using main of go-sdk

* Tweaks per PR

* Adding file headers and some tweaks per PR

* More tweaks

* More comments in Kafka test

Co-authored-by: tanvigour <60332928+tanvigour@users.noreply.github.com>
Co-authored-by: Tanvi Gour <tanvi.gour@gmail.com>
Co-authored-by: tanvigour <>
Co-authored-by: Long Dai <long.dai@intel.com>
Co-authored-by: Simon Leet <31784195+CodeMonkeyLeet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants